-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#4876] Add support for modifier keys during drag operations #4879
base: 4.2.x
Are you sure you want to change the base?
Conversation
Adds the ability to use the OS-defined modifier key (usually Alt on Windows and Option on Mac) to toggle between the default drop behavior and the opposite behavior. So if dragging within the same actor this changes from the default move behaior to copy behavior and the opposite when dragging between different actors or to the sidebar. Enabling this required access to the current drag payload during the `ondragover` event, so this extends the `DragDrop` class provided by core to store that information during the `ondragstart` event and adds a new handler for the `ondragend` event to clear the stored payload. Currently this only covers dragging items, with some minor improvements to dragging favorites on the character sheet. This framework could be expanded in the future to support dragging actors into the bastion tab as well as dragging active effects, advancements, or activities. Closes #4876
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some high-level comments before I do a full review: I think it would be preferable to have fixed modifier keys that map to a specific action, rather than a key that toggles to the opposite of the default action. For example, holding Ctrl should always copy, and holding Shift should always move (doesn't have to be those exact modifiers), regardless of what the default action is.
Hmm, the thing I like about this approach is it takes advantage of the keys people already use with their operating system, so nothing new to learn (it also keeps the cursor updated so it is always clear what action will be performed). |
Not sure what you mean. Ctrl is copy and Shift is move in Windows. Alt is create link. So holding Alt to switch between move or copy depending on what the default action is would be new behaviour. |
Option-drag is always "duplicate" on macOS; there are no other OS-wide drag modifiers here. For what it's worth :) |
Okay, I've done a bit more research and here is what is provided by OSes: Windows
Mac
So we could setting on something that works for both:
Since I don't have access to my Windows machine at the moment I haven't been able to test this to see how the current behavior works there. |
Ehhh I'm in full agreement on Option/Alt means copy, but Command doesn't actually have any relation to "move" on macOS, and is already overloaded as the Control analogue in Foundry. Also this is all confused by the fact that drag moves are already Copy events by default, so I'm not really sure what the imagined workflow is here. If you want to match the common behaviour from almost everywhere in the computing world, unmodified drags would be moves, while modified drags would be copies. But that's contrary to how core Foundry handles document drags (for fairly good reasons); not sure if flipping this upside down in the system would be a good idea. |
Command changes a copy-as-default drag to a move drag when moving between different drives on Mac OS. This is similar to Windows, where any drag-drop that is considered "free" (aka doesn't move between drives or computers) is move-by-default, while any drag-drop that is considered expensive is copy-by-default.
Drags are copy by default if moving from one actor to another, between the sidebar and an actor, and to or from a compendium. Drags are move by default if moving within an actor or moving into or out of a container on the same actor/sidebar/compendium. This would offer a way to change the behavior in all of those locations using modifier keys. |
Sure, but that's just a Finder shortcut; isn't used anywhere else, or expected behaviour in Mac apps. I do get where you're going with this, that we need a way to "do the other thing". But the challenge here is the tension between Kim's (reasonable) desire to have each modifier do a specific action, versus the fact that there are currently different behaviours in different contexts. My two cents on this would be: The Alt/Option key is generally considered a "do an alternate behaviour" modifier, so would make intuitive sense to have it reverse whatever the default behaviour of a given drag is. |
There are different behaviours in different contexts, and the proposal is that you may hold a modifier to force a specific behaviour that you intend that overrides the default behaviour. If you want to copy, you hold Ctrl, regardless of whether the default behaviour was a move or a copy. If you want to move, you hold Shift. You don't need to know what the default behaviour is beforehand, or take care to watch your cursor icon to see what the result of your modifier + context combo is. You only need to learn that you hold Ctrl if you intend to copy and hold Shift if you intend to move. |
Yeah I can get behind that :) Worth making sure this isn't going to collide with potential core modifiers — I know it's been suggested that Control-drag might be used to make token drags into teleports, for example, but perhaps in the future it might be nice to be able to modifier-drag tokens to create duplicates, so using the same key globally for that concept would be best. (also isn't this really a core-level concept regardless?) |
Adds the ability to use the OS-defined modifier key (usually Alt on Windows and Option on Mac) to toggle between the default drop behavior and the opposite behavior. So if dragging within the same actor this changes from the default move behaior to copy behavior and the opposite when dragging between different actors or to the sidebar.
Enabling this required access to the current drag payload during the
ondragover
event, so this extends theDragDrop
class provided by core to store that information during theondragstart
event and adds a new handler for theondragend
event to clear the stored payload.Currently this only covers dragging items, with some minor improvements to dragging favorites on the character sheet. This framework could be expanded in the future to support dragging actors into the bastion tab as well as dragging active effects, advancements, or activities.
Closes #4876